Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX #4279 CLI support multiline #4807

Merged
merged 1 commit into from
Nov 19, 2015
Merged

FIX #4279 CLI support multiline #4807

merged 1 commit into from
Nov 19, 2015

Conversation

flisky
Copy link
Contributor

@flisky flisky commented Nov 16, 2015

No description provided.

@otoolep
Copy link
Contributor

otoolep commented Nov 16, 2015

Thanks @flisky -- have you signed the CLA?

Can you provide a reference for this call?

@flisky
Copy link
Contributor Author

flisky commented Nov 17, 2015

Yes, I have signed the CLA with github name flisky.

@otoolep
Copy link
Contributor

otoolep commented Nov 17, 2015

Can you point me at a GoDoc reference for that call?

@otoolep
Copy link
Contributor

otoolep commented Nov 17, 2015

CLA confirmed signed.

@flisky
Copy link
Contributor Author

flisky commented Nov 17, 2015

@gunnaraasen
Copy link
Member

@otoolep GoDoc reference is here. It's a recent addition to the liner package authored by @flisky. Seems like the test error is unrelated to this change.

@otoolep
Copy link
Contributor

otoolep commented Nov 17, 2015

@corylanou

@otoolep
Copy link
Contributor

otoolep commented Nov 17, 2015

If it works, great.

@flisky -- can you show an example session of your CLI in action, with this change in place?

@otoolep
Copy link
Contributor

otoolep commented Nov 17, 2015

@gunnaraasen -- do you think you could clone the repo from @flisky and build his CLI? We'd like to see this in action before merging.

@flisky
Copy link
Contributor Author

flisky commented Nov 18, 2015

Here is a commandline session, and please let me know if anything I missed.

There are two problems I knew -

  1. PR multiline follow-up: reset state after enter key pressed peterh/liner#60 to fix multiline in repl loop - merged.
  2. WINCH signal is not handled perfectly, but I think it's acceptable.

@gunnaraasen
Copy link
Member

Thanks for the video @flisky. I can also confirm multiline wrapping works as expected.

screen shot 2015-11-18 at 6 50 30 pm

(also found out Github has a neat trick where PR branches can be checkout out directly using git fetch origin pull/4807/head:<NEW-BRANCH-NAME>)

@otoolep I think this PR is ready to merge.

otoolep added a commit that referenced this pull request Nov 19, 2015
@otoolep otoolep merged commit 1e9e7c2 into influxdata:master Nov 19, 2015
@otoolep
Copy link
Contributor

otoolep commented Nov 19, 2015

Thanks @gunnaraasen

@flisky
Copy link
Contributor Author

flisky commented Nov 19, 2015

Thanks for the tip, @gunnaraasen, and thanks for the merging, @otoolep:)

@flisky
Copy link
Contributor Author

flisky commented Nov 20, 2015

Hi there, may I ask for a t-shirt? Thanks,

@otoolep
Copy link
Contributor

otoolep commented Nov 20, 2015

Sure! Email me your address.

On Thursday, November 19, 2015, 尹吉峰 notifications@github.com wrote:

Hi there, may I ask for a t-shirt? Thanks,


Reply to this email directly or view it on GitHub
#4807 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants